Enable serialization of the connection #264
Conversation
|
Thanks @richt222. Does this work with queued jobs immediately when adding this trait or is there still some work that needs to be done? |
|
Hey @joedixon, It works right away with no extras required. Tested with a DB queue. It was just the fact that the connection object could not be serialised directly in its current state. Adding the |
| @@ -15,7 +15,6 @@ public function __serialize(): array | |||
| { | |||
| return [ | |||
| 'id' => $this->id(), | |||
| 'identifier' => $this->identifier(), | |||
There was a problem hiding this comment.
Please mark as ready for review when updated.
There was a problem hiding this comment.
identifier isn't a direct property of the Connection object being serialized, but was incorrectly being set as such when the object gets unserialized. This was causing PHPStan to fail static analysis.
The socket's raw identifier is resolved from the underlying WebSocketConnection which is passed in Connection's constructor. For this reason, it was removed from the Connection serialization, which currently only stores the "normalized" socket ID as one of its properties.
Maybe this PR needs more work to try and resurrect the entire connection when unserialized. Although that could be quite a rabbit hole.
|
I dug into this some more and it's not actually possible to serialize the underlying connection, meaning you can't actually send any message to the connection once it's been unserialized. So, going to close this one since I don't think serializing it is providing much value. |
|
Thanks @joedixon. Yep, I agree. I also spent a good while trying to figure out if the underlying connection could be serialized and I think, although possible, it would be a big task and ultimately prone to failures. The only slight frustration is that common events such as MessageSent can't be automatically queued as the event includes the connection. So listeners either require an extra step to create the queued job, minus the connection, or they get handled synchronously, which can obviously block the event loop. |
|
So, event though the SerializesConnections exist, it's not implemented on the Connection, which means that the listeners bound to Reverb events cannot be queued. So, out of the box, there's no way to queue the listener for the MessageReceived event to parse/process the json payload and handle it however, without creating and manually dispatching a custom job from the "sync" listener. Without diving too deep, is there a reason why the MessageReceived also has the Connection object passed along with stringified message? Could we maybe have more events to handle just the message, or at least subscribe/unsubscribe events? Thanks :) |
Adds the
SerializesConnectionstrait to theConnectionclass, enabling its serialization.It is sometimes necessary to serialize the connection, as it is dispatched in common events such as
MessageSentwhich may need storage. Listeners may wish to automatically queue the handling of such events by implementingShouldQueue. This currently results in a "serialization of 'closure' is not allowed" error, as the connection object passed in the event is not serializable.A workaround is to manually create the queue job in the listener's handler, by first parsing the connection object. However this sometimes isn't ideal and adds complexity.
In the context of Reverb, I think it's particularly important the handling of events are easily queued to prevent blocking of the event loop.